Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

stripping out new analytics to return things back to normal closes #4315 #4316

Merged
merged 4 commits into from
May 17, 2016
Merged

stripping out new analytics to return things back to normal closes #4315 #4316

merged 4 commits into from
May 17, 2016

Conversation

myleshorton
Copy link
Contributor

No description provided.

@myleshorton
Copy link
Contributor Author

This just strips out most of the changes in 2.2.x to return analytics back to their previous state. This reflects several things, including that non-interaction hits are handled very differently (apparently) and that the Lantern version actually proved to be very useful in identifying fake versions.

@myleshorton
Copy link
Contributor Author

Hey @oxtoacart you mind taking a look at this -- I just realized our analytics are still pretty messed up!

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 52.0% when pulling 6972b66 on issue-4315 into b1ac4f5 on release-2.2.0.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 52.017% when pulling 6972b66 on issue-4315 into b1ac4f5 on release-2.2.0.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 51.966% when pulling 31476ae on issue-4315 into b1ac4f5 on release-2.2.0.

@myleshorton
Copy link
Contributor Author

It looks like the coverage changes are just based on randomly touched code in the tests @oxtoacart so tempted to ignore.

oxtoacart
oxtoacart reviewed May 17, 2016
View reviewed changes
src/github.com/getlantern/flashlight/analytics/analytics.go
transport func(string, eventual.Getter)) func() {
var addr atomic.Value
go func() {
logging.AddUserAgentListener(func(agent string) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@myleshorton With this change, the AddUserAgentListener capability is no longer used. Can you remove it and the corresponding test please?

@oxtoacart oxtoacart self-assigned this May 17, 2016
@myleshorton
Copy link
Contributor Author

Yup all set @oxtoacart!

@oxtoacart
Copy link
Contributor

Code LGTM, just waiting for tests to pass.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 51.891% when pulling 6ba8a00 on issue-4315 into b1ac4f5 on release-2.2.0.

@oxtoacart oxtoacart merged commit c4c75e0 into release-2.2.0 May 17, 2016
@oxtoacart oxtoacart deleted the issue-4315 branch May 17, 2016 18:25
@myleshorton
Copy link
Contributor Author

Thanks @oxtoacart! I'll add this on devel as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@oxtoacart oxtoacart

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@myleshorton @coveralls @oxtoacart